Redacting secret values in the configuration summary#1922
Conversation
📝 WalkthroughWalkthroughAdds redaction support to config-resolution summaries, exposes a redacted public wrapper, switches compose upload to call the redacted variant, changes secret detection to return detector types and tighter rules, updates callers, and adds extensive redaction-focused testdata and tests. Changes
Sequence Diagram(s)(Skipped) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/pkg/cli/testdata/redact-config/high-entropy-strings/compose.yaml.golden`:
- Around line 4-8: The ENCRYPTION_KEY value is not being redacted even though it
matches the high-entropy and keyword rules used for SESSION_SECRET and
SIGNING_KEY; update the secret-detection/redaction logic so keys containing
"KEY" (e.g., ENCRYPTION_KEY) get the same treatment as keys containing "SECRET"
by ensuring the keyword detector includes "KEY" (case-insensitive) or that the
high_entropy_string detector is applied regardless of key-name branching with
threshold=4, then regenerate the golden so the value becomes 9f86***; check
functions/modules named like high_entropy_string detector and the keyword
detector/whitelist handling where SESSION_SECRET and SIGNING_KEY are matched.
In `@src/pkg/cli/testdata/redact-config/secrets-in-env-file/.env`:
- Around line 1-4: The .env fixture contains realistic-looking secrets
(GITHUB_TOKEN, DATABASE_URL, POSTGRES_PASSWORD, API_SECRET) which will trigger
secret scanners; update the fixture values to clearly synthetic placeholders
that preserve key names and required formats for tests (e.g.,
GITHUB_TOKEN=ghp_FAKE_TOKEN, DATABASE_URL=postgres://user:pass@host:5432/dbname,
POSTGRES_PASSWORD=FAKE_PASSWORD, API_SECRET=FAKE_SECRET) or alternatively ensure
the testdata path is excluded from your secret-scanner config, then run the
redaction tests to confirm they still exercise the same code paths.
🧹 Nitpick comments (1)
src/pkg/cli/configResolution_test.go (1)
53-75: Consider adding defangConfigs cases for redacted tests.The new
TestPrintRedactedConfigResolutionSummarypassesnilfordefangConfigs(Line 63), whileTestPrintConfigResolutionSummaryhas explicit case-based handling for different test scenarios (Lines 26-37). If the redaction logic should also respect known defang config variables (in addition to pattern-based detection), you may want similar case handling here for completeness.If passing
nilis intentional because redaction relies solely on value/name pattern detection, this is fine as-is.
src/pkg/cli/testdata/redact-config/high-entropy-strings/compose.yaml.golden
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pkg/cli/configResolution.go`:
- Around line 36-42: The maskTrailingConfigValue function currently exposes
4-character secrets; change the logic so any value with length <= 4 is fully
masked (return configMaskedValue) instead of returning it unmasked, and for
values longer than 4 keep the visible prefix of the first 4 characters but mask
the rest (use strings.Repeat("*", len(value)-4) rather than a fixed 3 stars) to
avoid revealing suffixes—update maskTrailingConfigValue accordingly.
Description
This pull request introduces support for redacting secret values in the configuration summary output, particularly when displaying environment variables that are likely to contain sensitive data. The changes add a mechanism to mask secrets in summaries, update the relevant code paths to use this redacted summary when appropriate, and expand test coverage to ensure the new behavior works as intended.
We will now redact on every compose up but not when the user explicitly calls
defang config resolve.While added test case
9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08this hex did not pass the secret detector. Turn our we need to set our values to certain format which we chose yaml and slightly lower our entropy from 4 -> 3.7.Linked Issues
#1920
Checklist
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests